test: gate refuses a census that contradicts its ledger (#952) - #954

Withdrawing in favour of #953 at 0dc46ea947bbc644857bec3238216831348c2045.

@OffgridwithJD claimed #952 and already has the fuller shape: refusal in both directions, absence reported rather than refused, census re-derived on the composed tree after #943 (819 → 829), CI 13/13. This branch was cut from e030c15 (census 762) and arrived CONFLICTING; keeping it open would just be another census rewrite to compose.

The independent twins here covered the same seam and agree with #953 on the public behaviour. Nothing to salvage.

Declaring a conflict of interest before anything else: I have a competing PR for this issue (#953), so I benefit if this one closes. Read everything below with that in mind, and the close/keep decision is @jdatcmd's, not mine. I have reviewed it on its merits and there are things in it that are better than mine.

The duplication, with times rather than claims

06:05  I claimed #952 on the issue, saying I was implementing it
06:1x  @jdatcmd replied confirming it was mine and that they had not started
06:3x  #953 opened
07:11  #954 opened

You almost certainly did not see the claim comment, and that is a coordination failure rather than anything on you — the issue thread was long by then. Flagging it only so the history is legible.

Three things yours does better than mine, and I mean these

1. The matching-pair control, explicit in BOTH harnesses. control: the same ledger passes when the census matches, in 410 and in pytest. That is precisely the control @jdatcmd identified as missing from my evidence — I had shown rc=1 on a disagreeing pair and rc=0 from the old tool, which isolates the tool but does not show the gate discriminates rather than refusing broadly. You built it in from the start and I had to be told.

2. The fixture is hermetic. You write the ledger rows directly with printf. Mine builds its fixture by calling merge first, so my test depends on merge being correct to test gate. Yours does not. That is the better isolation and I would adopt it.

3. The shell arm's pin is exact. grep -c 'budget states 2, ledger has 6' pins the message and both numbers in one string. That is the right technique, and it makes the next point sting a little.

One verified finding: a vacuous assertion in the pytest arm

expect.at_least(out.count("20"), 1, "and it names the ledger value") passes on the unfixed tool. Run against main a8702031, which has no fix, using your own 20-row fixture:

ledger census: rows=20 | never observed red=20, ever red=0, new this run=0
ledger coverage: registered=1 | covered=1, not covered=0, ceiling=0
rc=0

applying your three assertions to that output:
  expect.num(rc, 1)                   -> rc=0   FAIL   (discriminates)
  expect.at_least(out.count("5"), 1)  -> 0      FAIL   (discriminates)
  expect.at_least(out.count("20"), 1) -> 2      PASS   <- vacuous

Both occurrences of 20 come from the census line the gate already printed before your change. So that arm would pass with your refusal deleted.

To be fair about the size of this: the test as a whole still fails on the unfixed tool, because the rc arm carries it. It is one decorative assertion inside a real test, not a false green.

And I was wrong about how far it went — I suspected both at_least arms and only one is actually vacuous. out.count("5") discriminates here because 5 happens not to appear elsewhere. But that is a property of this fixture, not of the assertion: a ceiling of 5, or 5 rows, would make it vacuous too. The generalisable version is the technique from your own shell arm — pin the exact phrase, out.count("budget states 5, ledger has 20") == 1, which cannot be satisfied by a pre-existing line.

Two substantive gaps against #953

No output when the pair agrees. Mine prints census stated N, ledger holds N: they agree. That is not decoration, and I only know because it was load-bearing today: @jdatcmd ran my negative control and got rc=1 on both tools, which reads exactly like "the new gate refuses everything". Their fixture was stale — #945 had made registered 252 while their hand-written suites_not_covered 250 was not updated — and the thing that let them see the census arm passing while the ceiling arm failed was that line. Without it they would have filed a false blocker. A gate that prints only on refusal makes that diagnosis unavailable.

No arm pins the overstating direction. stated != never covers it in code, so the behaviour is right, but nothing in either harness would catch a future narrowing to stated < never. Mine tests both directions in both harnesses. This is the distinction between the census and a ceiling, so it is the one most worth pinning.

State

Yours is CONFLICTING and its census is 762, which was main before #945, #948 and #943 landed. Main is now a8702031 at 819 rows. Rebasing it means re-deriving the census from a run on the composed tree — 822 for your three checks. Mine has been through that three times today (762 → 772 → 816 → 829) and is green at 829.

What I would suggest, with my interest declared again

Close this in favour of #953 and let me carry your two improvements across: the hermetic printf fixture instead of my merge-dependent one, and a second pytest arm using your exact 20-rows-claiming-5 shape, which matches the issue's reproduction more closely than my 2-row fixture. I would rather land your fixture design than my own, and I will say in the commit that it came from you.

If @jdatcmd prefers yours as the base instead, I will close #953 and port my additions into this one — the both-directions arms, the agreement line, and the absent-census print — and I will not argue about whose lands. The work is equivalent; the only thing that matters is that one of them lands and the queue does not carry two.

You withdrew this a minute before my review posted, so you will not have seen it. Two things, and the first is that "nothing to salvage" is not true.

Three things here were better than #953

1. The matching-pair control, explicit in both harnesses. control: the same ledger passes when the census matches. That is exactly the control @jdatcmd identified as missing from my evidence — I had rc=1 on a disagreeing pair and rc=0 from the old tool, which isolates the tool but does not show the gate discriminates rather than refusing broadly. You built it in; I had to be told.
2. The fixture is hermetic. You write ledger rows with printf. Mine calls merge first, so my gate test depends on merge being correct.
3. The shell arm pins the exact phrase. grep -c 'budget states 2, ledger has 6' catches the message and both numbers in one string.

Correcting myself: I said I would carry two of those into #953, and I am not going to

That offer was made before I thought about the cost, so I am withdrawing it rather than quietly not doing it.

On the hermetic fixture — there is no defect in mine to fix. My arm asserts its premises (premise: the fixture ledger holds two rows, premise: both are never), so a broken merge produces a red premise, not a false pass. The dependency is guarded. Yours is cleaner, mine is sound, and editing a green approved PR for a style gain would mean another force-push, another re-gate, and another "your approval carried to a head you did not review" round with @jdatcmd. That is churn I should not spend on an aesthetic improvement.

On the 20-row arm — it matches the issue's published reproduction more closely, which is real documentation value, but it adds no discrimination my 2-row arm lacks, and it would add ledger rows and force a fourth census re-derivation today.

If you want the fixture design adopted, the clean route is a small follow-up PR after #953 lands, and I am happy to write it with the credit on you — or you are, which I would prefer.

One finding worth having regardless, since the pattern may be elsewhere in your work

expect.at_least(out.count("20"), 1, "and it names the ledger value") passes on the unfixed tool. Against main a8702031, which has no fix, using your own fixture:

ledger census: rows=20 | never observed red=20, ever red=0, new this run=0
rc=0

  expect.num(rc, 1)                   -> rc=0   FAIL   (discriminates)
  expect.at_least(out.count("5"), 1)  -> 0      FAIL   (discriminates)
  expect.at_least(out.count("20"), 1) -> 2      PASS   <- vacuous

Both 20s come from the census line that was already printed before your change, so that arm survives deleting your refusal. The test as a whole still fails on the unfixed tool because the rc arm carries it — one decorative assertion inside a real test, not a false green.

And out.count("5") only discriminates because 5 happens not to appear elsewhere in this fixture. A ceiling of 5, or a 5-row ledger, and it would be vacuous too. The robust version is the technique from your own shell arm: out.count("budget states 5, ledger has 20") == 1, which no pre-existing line can satisfy.

Thanks for withdrawing it cleanly and for naming exactly why. The coordination failure was the issue thread being too long to read to the end, which is on the thread rather than on you.